Incidents Backend Refactor #164
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Test Results Summary293 tests - 9 293 ✅ - 9 11s ⏱️ ±0s Results for commit 33cd7dc. ± Comparison against base commit c972c7c. This pull request removes 44 and adds 35 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
aLEGEND21
changed the title
naasanov
requested changes
Jan 30, 2026
Collaborator
naasanov
left a comment
naasanov
left a comment
There was a problem hiding this comment.
Looks great, just a couple changes
Comment on lines
16
to
31
| test_incident_authentication = generate_auth_required_tests( | ||
| ({"admin", "staff"}, "GET", "/api/locations/1/incidents", None), | ||
| ( | ||
| {"admin"}, | ||
| "POST", | ||
| "/api/locations/1/incidents", | ||
| IncidentTestUtils.get_sample_data(), | ||
| ), | ||
| ( | ||
| {"admin"}, | ||
| "PUT", | ||
| "/api/locations/1/incidents/1", | ||
| IncidentTestUtils.get_sample_data(), | ||
| ), | ||
| ({"admin"}, "DELETE", "/api/locations/1/incidents/1", None), | ||
| ) |
Collaborator
There was a problem hiding this comment.
Missed this in the TDD but police should have access to all incident routes
Collaborator
Author
There was a problem hiding this comment.
Updated. Although, I'm getting the following error when running the test suite:
________________________ test_incident_authentication[allowed_roles1-POST-/api/locations/1/incidents-body1] ________________________
create_test_client = <function create_test_client.<locals>._create_test_client at 0xffffa1df2ca0>
allowed_roles = {'admin', 'police'}, method = 'POST', path = '/api/locations/1/incidents'
body = {'description': 'Incident 0', 'incident_datetime': '2026-01-01T00:00:00+00:00', 'location_id': 1, 'severity': 'complaint'}
@pytest.mark.parametrize("allowed_roles, method, path, body", params)
@pytest.mark.asyncio
async def test_authentication(
create_test_client: Callable[..., AsyncGenerator[AsyncClient, Any]],
allowed_roles: set[StringRole],
method: str,
path: str,
body: dict | None,
):
"""Test authentication and authorization for endpoints."""
for role in allowed_roles:
async for client in create_test_client(role):
> response = await client.request(method, path, json=body)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
test/utils/http/test_templates.py:40:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
/usr/local/lib/python3.12/site-packages/httpx/_client.py:1540: in request
return await self.send(request, auth=auth, follow_redirects=follow_redirects)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/local/lib/python3.12/site-packages/httpx/_client.py:1629: in send
response = await self._send_handling_auth(
/usr/local/lib/python3.12/site-packages/httpx/_client.py:1657: in _send_handling_auth
response = await self._send_handling_redirects(
/usr/local/lib/python3.12/site-packages/httpx/_client.py:1694: in _send_handling_redirects
response = await self._send_single_request(request)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/local/lib/python3.12/site-packages/httpx/_client.py:1730: in _send_single_request
response = await transport.handle_async_request(request)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/local/lib/python3.12/site-packages/httpx/_transports/asgi.py:170: in handle_async_request
await self.app(scope, receive, send)
/usr/local/lib/python3.12/site-packages/fastapi/applications.py:1135: in __call__
await super().__call__(scope, receive, send)
/usr/local/lib/python3.12/site-packages/starlette/applications.py:107: in __call__
await self.middleware_stack(scope, receive, send)
/usr/local/lib/python3.12/site-packages/starlette/middleware/errors.py:186: in __call__
raise exc
/usr/local/lib/python3.12/site-packages/starlette/middleware/errors.py:164: in __call__
await self.app(scope, receive, _send)
/usr/local/lib/python3.12/site-packages/starlette/middleware/cors.py:85: in __call__
await self.app(scope, receive, send)
/usr/local/lib/python3.12/site-packages/starlette/middleware/exceptions.py:63: in __call__
await wrap_app_handling_exceptions(self.app, conn)(scope, receive, send)
/usr/local/lib/python3.12/site-packages/starlette/_exception_handler.py:53: in wrapped_app
raise exc
/usr/local/lib/python3.12/site-packages/starlette/_exception_handler.py:42: in wrapped_app
await app(scope, receive, sender)
/usr/local/lib/python3.12/site-packages/fastapi/middleware/asyncexitstack.py:18: in __call__
await self.app(scope, receive, send)
/usr/local/lib/python3.12/site-packages/starlette/routing.py:716: in __call__
await self.middleware_stack(scope, receive, send)
/usr/local/lib/python3.12/site-packages/starlette/routing.py:736: in app
await route.handle(scope, receive, send)
/usr/local/lib/python3.12/site-packages/starlette/routing.py:290: in handle
await self.app(scope, receive, send)
/usr/local/lib/python3.12/site-packages/fastapi/routing.py:115: in app
await wrap_app_handling_exceptions(app, request)(scope, receive, send)
/usr/local/lib/python3.12/site-packages/starlette/_exception_handler.py:53: in wrapped_app
raise exc
/usr/local/lib/python3.12/site-packages/starlette/_exception_handler.py:42: in wrapped_app
await app(scope, receive, sender)
/usr/local/lib/python3.12/site-packages/fastapi/routing.py:101: in app
response = await f(request)
^^^^^^^^^^^^^^^^
/usr/local/lib/python3.12/site-packages/fastapi/routing.py:355: in app
raw_response = await run_endpoint_function(
/usr/local/lib/python3.12/site-packages/fastapi/routing.py:243: in run_endpoint_function
return await dependant.call(**values)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
src/modules/incident/incident_router.py:42: in create_incident
return await incident_service.create_incident(location_id, incident_data)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
src/modules/incident/incident_service.py:57: in create_incident
await self.session.commit()
/usr/local/lib/python3.12/site-packages/sqlalchemy/ext/asyncio/session.py:1000: in commit
await greenlet_spawn(self.sync_session.commit)
/usr/local/lib/python3.12/site-packages/sqlalchemy/util/_concurrency_py3k.py:190: in greenlet_spawn
result = context.switch(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/usr/local/lib/python3.12/site-packages/sqlalchemy/orm/session.py:2030: in commit
trans.commit(_to_root=True)
<string>:2: in commit
???
/usr/local/lib/python3.12/site-packages/sqlalchemy/orm/state_changes.py:101: in _go
self._raise_for_prerequisite_state(fn.__name__, current_state)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <sqlalchemy.orm.session.SessionTransaction object at 0xffffa1312c10>, operation_name = 'commit'
state = <SessionTransactionState.DEACTIVE: 4>
def _raise_for_prerequisite_state(
self, operation_name: str, state: _StateChangeState
) -> NoReturn:
if state is SessionTransactionState.DEACTIVE:
if self._rollback_exception:
> raise sa_exc.PendingRollbackError(
"This Session's transaction has been rolled back "
"due to a previous exception during flush."
" To begin a new transaction with this Session, "
"first issue Session.rollback()."
f" Original exception was: {self._rollback_exception}",
code="7s2a",
)
E sqlalchemy.exc.PendingRollbackError: This Session's transaction has been rolled back due to a previous exception during flush. To begin a new transaction with this Session, first issue Session.rollback(). Original exception was: (sqlalchemy.dialects.postgresql.asyncpg.IntegrityError) <class 'asyncpg.exceptions.ForeignKeyViolationError'>: insert or update on table "incidents" violates foreign key constraint "incidents_location_id_fkey"
E DETAIL: Key (location_id)=(1) is not present in table "locations".
E [SQL: INSERT INTO incidents (location_id, incident_datetime, severity, description) VALUES ($1::INTEGER, $2::TIMESTAMP WITH TIME ZONE, $3::incidentseverity, $4::VARCHAR) RETURNING incidents.id]
E [parameters: (1, datetime.datetime(2026, 1, 1, 0, 0, tzinfo=TzInfo(0)), 'COMPLAINT', 'Incident 0')]
E (Background on this error at: https://sqlalche.me/e/20/gkpj) (Background on this error at: https://sqlalche.me/e/20/7s2a)
/usr/local/lib/python3.12/site-packages/sqlalchemy/orm/session.py:971: PendingRollbackError
------------------------------------------------------- Captured stdout call -------------------------------------------------------
Expecting authorized for admin client... ✓
===================================================== short test summary info ======================================================
FAILED test/modules/incident/incident_router_test.py::test_incident_authentication[allowed_roles1-POST-/api/locations/1/incidents-body1] - sqlalchemy.exc.PendingRollbackError: This Session's transaction has been rolled back due to a previous exception during flush. ...
================================================== 1 failed, 294 passed in 5.17s ===================================================```
Was able to fix it by rolling back the test session every time in conftest.py:
if test_session.in_transaction() and not test_session.is_active:
await test_session.rollback()
naasanov
requested changes
Jan 30, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current system tracks complaints as their own table while warnings and citations are stored as integer counters on the Location entity. This creates inconsistency in how we track incidents and makes it difficult to view the history or details of individual warnings/citations. This PR consolidates all three incident types into a single Incidents table with a severity field.
Changes:
New Incident Module
incident_entity.py,incident_model.py,incident_service.py, andincident_router.pyto replace the complaint moduleIncidentSeverityenum with values:complaint,warning,citationseverityfield to incident data model and entityLocation Entity Updates
warning_countandcitation_countcolumns since these are now derived from the incidents tablecomplaintsrelationship toincidentsLocationDtoto returnincidentslist instead ofcomplaintsMAX_COUNTconstant and count validation logicRemoved Increment Routes
POST /api/police/locations/{id}/warningsandPOST /api/police/locations/{id}/citationsendpointspolice_router.pysince police credential routes live inaccount_router.pyincrement_warnings()andincrement_citations()methods fromLocationServiceTest Updates
incident_utils.py,incident_service_test.py,incident_router_test.py)Script Updates
reset_dev.pyto removewarning_countandcitation_countfrom location seedingCursor was helpful with making these changes, but I reviewed all AI-made changes prior to committing the code.
Closes #127